Step3 #358
Step3 #358
Conversation
| import nextstep.courses.domain.session.Session; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import java.util.List; | ||
|
|
||
| public class Course { | ||
| public class Course extends BaseEntity { | ||
| private Long id; |
| } | ||
|
|
||
| public void decrease() { | ||
| this.remainCount--; | ||
| this.availableCount--; |
There was a problem hiding this comment.
availableCount이 총 수강 가능한 인원이라면 변하면 안 될 것 같아요. 원래 강의의 최대 수강 인원이 몇 명인지는 유지되어야 하지 않을까요?
remainCount와 수강 인원(Enrollment)가 중복이라서, remainCount 없이 Enrollment를 이용해도 되지 않을지 궁금해서 남긴 코멘트였습니다. :)
| return enrollmentCount; | ||
| } | ||
|
|
||
| public void validateEnrollState(Payment payment) { |
There was a problem hiding this comment.
이 메서드는 검증을 하는 목적이 아니라, 수강 신청을 하는 것(enrollmentCount.decrease())을 목적으로 호출하는 메서드일 것 같아요.
메서드 이름을 변경해보면 어떨까요?
| @@ -18,7 +20,32 @@ public boolean isFree() { | |||
| return fee == 0; | |||
| } | |||
|
|
|||
| public static SessionFee ZERO() { | |||
There was a problem hiding this comment.
fee가 final 이라서 불변으로 관리되니, ZERO는 상수로 만들어서 하나의 객체를 재사용해보면 어떨까요?
| return ps; | ||
| }, keyHolder); | ||
| Number key = keyHolder.getKey(); | ||
| return key != null ? key.longValue() : -1; |
There was a problem hiding this comment.
KeyHolder를 활용한 id 반환 좋습니다. 👍
저장이 안 된 경우 -1을 반환하신 이유가 있을까요?
문제가 발생한 상황이라면 예외를 발생시키는 건 어떨까요?
| return enrollmentRepository.save(enroll); | ||
| } | ||
|
|
||
| public Enrollment enroll(final Session session, final NsUser user, final Payment payment) { |
There was a problem hiding this comment.
20행의 enroll 메서드에서만 사용되는 메서드이니, private 접근 제어자를 사용해보면 어떨까요?
|
|
||
| @Service | ||
| public class EnrollmentService { |
There was a problem hiding this comment.
메서드에 @Transactional을 누락할 때 발생할 수 있는 문제를 방지하기 위해 클래스에도 @Transactional을 설정해보면 어떨까요?
|
|
||
|
|
||
| public Enrollment(final Long nsUserId, final Long sessionId) { | ||
| this(null, nsUserId, sessionId, LocalDateTime.now(), null); |
There was a problem hiding this comment.
생성자 내부에서 생성일시를 만들지 말고, 서비스 또는 외부에서 시간을 요청 받아보면 어떨까요?
현재의 구조에서는 한 트랜잭션 내부에서 여러 데이터를 생성하더라도, 각 데이터의 생성 일시가 달라질 것으로 보입니다.
안녕하세요 3단계 PR 및 승인 요청드립니다!
1단계 코드리뷰 남겨주신 부분은 추후 수정해놓겠습니다~!
감사합니다